-
Notifications
You must be signed in to change notification settings - Fork 67
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Updated parameter introspection and traj parameters #964
Conversation
_get_targets_metadata now returns metadata on a per target basis, and determining the appropriate shape of a control, state, or parameter is done outside of the function. brachistochrone and some other tests are working but a lot of work is left to be done. this should allow us to introspect the appropriate default shape and value for parameters and other variables.
…rameter has no targets since it may be used as a rate source.
dymos/trajectory/trajectory.py
Outdated
""" | ||
phase_param_options = {} | ||
for phs in self.phases._subsystems_myproc: | ||
phase_param_options.update({phs.name: phs.parameter_options}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Creating a temporary dict just to call update isn't necessary. Could just do phase_param_options[phs.name] = phs.parameter_options
dymos/trajectory/trajectory.py
Outdated
phase_param_options.update({phs.name: phs.parameter_options}) | ||
|
||
if self.comm.size > 1: | ||
data = self.comm.gather(phase_param_options, root=0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you need an allgather here. Otherwise only rank 0 contains the full data and all other procs only contain their local data.
dymos/utils/introspection.py
Outdated
|
||
# Check that all targets have the same shape. | ||
static_shapes = {} | ||
dynamic_shapes = {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you need 2 different dicts here since you combine them together later anyway?
dymos/utils/introspection.py
Outdated
ValueError | ||
ValueError is raised if the targets do not all have the same metadata value. | ||
""" | ||
meta_set = {meta[metadata_key] for tgt, meta in targets.items()} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You only need the metadata here, not the target name, so could just use targets.values()
dymos/utils/introspection.py
Outdated
else: | ||
introspected_shape = None | ||
|
||
if options['shape'] in {_unspecified, None}: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You use {_unspecified, None}
or {None, _unspecified}
in quite a few places in dymos. Would it be worth it to define a global constant with that value to avoid creating all of those temporary sets?
Summary
Trajectory parameters now pull introspection data from constituent phases even under MPI.
Phase parameters now allow for a mixture of static and dynamic targets.
Deprecated
static_target
in favor ofstatic_targets
which is a set of those targets which are static.**Previously option
static_target
was an all-or-nothing setting for a parameter.Now option
static_targets
is a set of targets to which the connection is treated as a static one (not sized with the number of nodes in the phase.Dymos will use introspection to attempt to determine which targets are static.
static_targets
may still be provided as an "all-or-nothing"True
orFalse
option.Changes to internal method
_get_targets_metadata
The method now returns a dictionary keyed by target path and associated with metadata (shape, units, value, tags) of the given target.
Added new introspection method
_get_common_metadata
This method will accept targets and a metadata key, and then return the value of the given metadata key if it is shared across all targets. This is useful when trying to infer the units or shape of a parameter.
Changes to
ODEEvaluationGroup
Under the
ExplicitShooting
transcription,ODEEvaluationGroup
is required to perform its own introspection since its configure method occurs before that of the parent phase. This group is now passed copies of the time, state, control, and parameter options dictionaries of the parent phase, so that results of its introspection will not conflict with those of the parent phase.Related Issues
Backwards incompatibilities
None
New Dependencies
None